✨ Promote Webhook FeatureGates to GA (OPRUN-4098) #2267

camilamacedo86 · 2025-10-14T11:01:00Z

No description provided.

netlify · 2025-10-14T11:01:53Z

✅ Deploy Preview for olmv1 ready!

Name	Link
🔨 Latest commit	`e4aaf82`
🔍 Latest deploy log	https://app.netlify.com/projects/olmv1/deploys/68f5fca96e02db0008c19b69
😎 Deploy Preview	https://deploy-preview-2267--olmv1.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

docs/draft/howto/enable-webhook-support.md

codecov · 2025-10-14T11:31:26Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 70.14%. Comparing base (292c0db) to head (e4aaf82).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2267      +/-   ##
==========================================
- Coverage   73.00%   70.14%   -2.86%     
==========================================
  Files          88       88              
  Lines        8743     8743              
==========================================
- Hits         6383     6133     -250     
- Misses       1947     2194     +247     
- Partials      413      416       +3

Flag	Coverage Δ
e2e	`45.23% <ø> (+5.81%)`	⬆️
experimental-e2e	`14.30% <ø> (-32.50%)`	⬇️
unit	`58.11% <ø> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

test/e2e/webhook_support_test.go

docs/draft/howto/enable-webhook-support.md

docs/draft/tutorials/explore-available-content-metas-endpoint.md

docs/tutorials/explore-available-content.md

tmshort · 2025-10-14T13:47:08Z

/hold
We can't have both feature gates enabled simultaneously, as one is meant for upstream (WebhookProviderCertManager), and the other is meant for downstream (WebhookProviderOpenshiftServiceCA).

tmshort · 2025-10-14T15:41:13Z

I think my preference would be to leave the feature-gates off by default, and put just the one we want into the helm/olmv1/values.yaml file, but this would mean also leaving them in the experimental.yaml file as well, which is not desirable (i.e. have it in two places). The alternative is to use an if/.Values.openshift.enabled/else sequence to separate the two flags.

BUT, the strangeness comes from the fact that we have cluster-olm-operator getting feature-gates from Openshift. I have a downstream change that would "remove" all upstream gates from values.yaml, and that might be needed before this is merged downstream.

BUT, another strangeness occurs because helm does not merge/combine lists; they are overwritten.

I need to think about this some more.

camilamacedo86 · 2025-10-14T15:43:01Z

/hold

Until we decide how to do

tmshort · 2025-10-14T16:08:50Z

That we have two feature-gates that can't be enabled simultaneously is causing us this headache (this should not be an issue with 99%of other features).

Although the CertManager one takes priority:

operator-controller/cmd/operator-controller/main.go

Lines 517 to 524 in 1d685b1

    
           func getCertificateProvider() render.CertificateProvider { 
        
           	if features.OperatorControllerFeatureGate.Enabled(features.WebhookProviderCertManager) { 
        
           		return certproviders.CertManagerCertificateProvider{} 
        
           	} else if features.OperatorControllerFeatureGate.Enabled(features.WebhookProviderOpenshiftServiceCA) { 
        
           		return certproviders.OpenshiftServiceCaCertificateProvider{} 
        
           	} 
        
           	return nil 
        
           }

Which I guess is a reasonable default. But it means we have to explicitly turn off CertManager to get OpenshiftServiceCA, and we don't really have that capability downstream.

Due to the downstream mapping of feature-gates, we only handle "enabled" flags (i.e. we can't turn things off), and we couldn't turn things off without a way in the helm chart to do so (i.e. additional upstream effort).

And disabling features just sounds wrong to me. I really think we need to enable features.

Add in the fact that helm doesn't merge lists makes this more challenging.

I'm envisioning a number of possible solutions, none of which I really like, so I'm wondering if this is something to be discussed in a meeting (office hours)?

tmshort · 2025-10-14T16:18:25Z

Options?

Keep these feature-gates off, explicitly turn on CertManager upstream through a new standard.yaml file, which means that experimental.yaml would also need to include it. Downstream, the OpenshiftServiceCA value would be enabled via the Openshift feature-gates.

1a. Add a new "standardFeatures" list that parallels the existing helm feature lists so that the value doesn't have to be duplicated in both standard.yaml and experimental.yaml. (this is a bunch of effort to avoid duplication of a value in a list).

Change the feature-gates to be on, but then add upstream and downstream and cluster-olm-operator mechanisms to be able to disable the feature gates. This would be a lot of additional work.
Add a new --webhook-provider option to operator-controller to select between CertManager and OpenshiftServiceCA (the default would be no provider). This effectively replaces the feature-gates with a tri-state, and would be enabled in the chart via .Values.openshift.enabled or .Values.certManager.enabled settings.

cmd/operator-controller/main.go

helm/olmv1/templates/deployment-olmv1-system-operator-controller-controller-manager.yml

cmd/operator-controller/main.go

tmshort · 2025-10-15T13:49:59Z

There is no default certificate provider in OLMv1. We provide explicit options to turn on and use cert-manager and/or Openshift serviceca, and I think we need to be able to reflect that.

Some fairly small changes would be needed to make the --certificate-provider a true tri-state.

internal/operator-controller/features/features.go

Promote to GA WebhookProviderOpenshiftServiceCA and WebhookProviderCertManager. For upstream WebhookProviderCertManager is used by default when WebhookProviderOpenshiftServiceCA is disabled by default.

camilamacedo86 · 2025-10-17T06:57:17Z

Hey folks 👋

@perdasilva @tmshort

I’ve updated this PR — it now reflects what we discussed.
It looks good to merge once we get the green light from you and downstream is ready to receive it.

Please feel free to review when you get a chance 🙏

michaelryanpeter

Some suggestions for clarity. Feel free to apply what makes sense and disregard any comments that feel off topic.

docs/draft/howto/enable-webhook-support.md

michaelryanpeter · 2025-10-17T15:59:41Z

docs/draft/howto/enable-webhook-support.md

+OLMv1 supports the installation of bundles containing webhooks by default.
+The controller uses the `WebhookProviderCertManager`
+feature-gate unless you override it. To switch to the OpenShift Service CA provider,
+start the controller with `--feature-gates=WebhookProviderCertManager=false` and enable `--feature-gates=WebhookProviderOpenshiftServiceCA=true`.


Suggested change

OLMv1 supports the installation of bundles containing webhooks by default.

The controller uses the `WebhookProviderCertManager`

feature-gate unless you override it. To switch to the OpenShift Service CA provider,

start the controller with `--feature-gates=WebhookProviderCertManager=false` and enable `--feature-gates=WebhookProviderOpenshiftServiceCA=true`.

OLM v1 supports installing bundles that use admission webhooks.

By default, OLM v1 uses the community Cert Manager package for admission webhook validation. To use the OpenShift Service CA provider, set the `--feature-gates=WebhookProviderOpenshiftServiceCA=true` flag at startup.

We have more than one kind of webhook that is supported.
certManager is used only for the admission webhook
So, I think it would not be very accurate.

The other suggestion was accepted :-)

I'm confused, isn't that what I wrote?
"By default, OLM v1 uses the community Cert Manager package for admission webhook validation."

(I'm totally fine if you don't take the suggestion, but I thought we are both saying the same thing?)

We need to have "OLMv1 supports the installation of bundles containing webhooks by default."
(because we support more than admission webhooks)
Then, below, we explain that for the specific case, admission, certmanager is used.

So, I updated it with a mix of your suggestions.

Co-authored-by: Michael Peter <[email protected]>

docs/draft/howto/enable-webhook-support.md

camilamacedo86 · 2025-10-20T16:07:02Z

/hold cancel

@tmshort @perdasilva as we discussed lets merge this one first.
Waiting for your approvals.

camilamacedo86 · 2025-10-20T16:07:22Z

/hold cancel

tmshort · 2025-10-20T17:28:03Z

/lgtm
/approve
/override codecov/project

openshift-ci · 2025-10-20T17:28:10Z

@tmshort: Overrode contexts on behalf of tmshort: codecov/project

In response to this:

/lgtm
/approve
/override codecov/project

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

openshift-ci · 2025-10-20T17:28:11Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: tmshort

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [tmshort]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

camilamacedo86 requested a review from a team as a code owner October 14, 2025 11:01

openshift-ci bot requested review from pedjak and trgeiger October 14, 2025 11:01

camilamacedo86 commented Oct 14, 2025

View reviewed changes

docs/draft/howto/enable-webhook-support.md Outdated Show resolved Hide resolved

camilamacedo86 requested review from michaelryanpeter, perdasilva and tmshort October 14, 2025 11:33

camilamacedo86 changed the title ~~✨ Promote Webhook FeatureGates to GA~~ ✨ Promote Webhook FeatureGates to GA (OPRUN-4098) Oct 14, 2025

tmshort reviewed Oct 14, 2025

View reviewed changes

test/e2e/webhook_support_test.go Show resolved Hide resolved

tmshort reviewed Oct 14, 2025

View reviewed changes

docs/draft/howto/enable-webhook-support.md Outdated Show resolved Hide resolved

tmshort reviewed Oct 14, 2025

View reviewed changes

docs/draft/howto/enable-webhook-support.md Outdated Show resolved Hide resolved

tmshort reviewed Oct 14, 2025

View reviewed changes

docs/draft/tutorials/explore-available-content-metas-endpoint.md Outdated Show resolved Hide resolved

tmshort reviewed Oct 14, 2025

View reviewed changes

docs/tutorials/explore-available-content.md Outdated Show resolved Hide resolved

camilamacedo86 requested a review from tmshort October 14, 2025 13:32

openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 14, 2025

tmshort mentioned this pull request Oct 14, 2025

✨ Promote Single Own Feature Gate AND Config spec in the CR to GA (OPRUN-4098) #2268

Open

4 tasks

camilamacedo86 force-pushed the promote-webhook-ga branch 2 times, most recently from 562998c to 61ead05 Compare October 15, 2025 07:43